Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Land surface upgrades for HR2 #1777

Merged
merged 34 commits into from
Jul 19, 2023

Conversation

HelinWei-NOAA
Copy link
Collaborator

@HelinWei-NOAA HelinWei-NOAA commented Jun 2, 2023

Description

This PR will includes all land surface upgrades for HR2. The detail can be found from ccpp-physics PR #78. The answer of all suite running NoahMP will be changed. The new soil color data has to be copied over to @[INPUTDATA_ROOT]/FV3_fix_tiled to run RTs

Input data additions/changes

  • No changes are expected to input data.
  • Changes are expected to input data:
    • New input data.
    • Updated input data.
      The new soil color data can be copied from /scratch2/NCEPDEV/stmp1/Sanath.Kumar/repo_viirs30s_bnuv2

Anticipated changes to regression tests:

  • Changes are expected to the following tests:

RegressionTests_hera.log

Subcomponents involved:

  • AQM
  • CDEPS
  • CICE
  • CMEPS
  • CMakeModules
  • FV3
  • GOCART
  • HYCOM
  • MOM6
  • NOAHMP
  • WW3
  • stochastic_physics
  • none

Combined with PR's (If Applicable):

Commit Queue Checklist:

  • Link PR's from all sub-components involved in section below
  • Confirm reviews completed in ALL sub-component PR's
  • Add all appropriate labels to this PR.
  • Run full RT suite on either Hera/Cheyenne AND attach log to a PR comment.
  • Add list of any failed regression tests to "Anticipated changes to regression tests" section.

Linked PR's and Issues:

Testing Day Checklist:

  • This PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR.
  • Move new/updated input data on RDHPCS Hera and propagate input data changes to all supported systems.

Testing Log (for CM's):

  • RDHPCS
    • Hera
    • Orion
    • Jet
    • Gaea
    • Cheyenne
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
    • Completed
  • opnReqTest
    • N/A
    • Log attached to comment

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA As we discussed with @barlage, I have just started to work on component version of the NoahMP model to reflect these changes in https://github.com/NOAA-EMC/noahmp side. Did you tried to run component land RTs before? I am expecting that they will fail with this version of model. Anyway, once I have mods for the component model, I'll let you know.

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA I am not sure but we might also need to update the FMS interface of the component model since it is complaining about missing mpp_write_meta()call. Maybe FMS version is changed and I am not sure at this point how hard is to bring those changes to the component model. @barlage since we completely restructure the I/O layer of the component model and get rid of FMS dependency, do you think we need to spend time to fix old version of FMS I/O routines?

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA I think those errors are related with using r4 version of the FMS. I think it is fine since this version of code does not support -D32BIT=ON but new version could since we are not using FMS for I/O.

@barlage
Copy link
Collaborator

barlage commented Jun 8, 2023

@uturuncoglu don't know if I understand the question, if we don't fix this, how do we pass the regression test? I'd of course prefer not to spend any unnecessary effort, especially if this will change in the next component model version.

@uturuncoglu
Copy link
Collaborator

@barlage that is fine I think I could fix it easily. Sorry about confusion.

@uturuncoglu
Copy link
Collaborator

@barlage Couple of questions:

  • How we need to handle extra arguments in noahmpdrv() like canopy_heat_storage_ccppetc. I think those are outputs used by ccpp and we don't need them in the component model. Right? If so, I just need to add bunch of dummy arrays to satisfy the component model. BTW, it seems that thay are coming after errmsg, errflg, arguments which is not following the convention.

  • As I know that there is a soil color argument in the driver too and I think it is coming from FV3. Do we have tiled dataset that we could read this information through the NUOPC cap. If so, is these tied files are coupled to the run directory?

@barlage
Copy link
Collaborator

barlage commented Jun 12, 2023

@uturuncoglu

  • those terms after errmsg, errflg are optional, that's why they are at the end of the call. they are not needed for the component model, only for us to have more access to noahmp subtile fields if we need them. at some point, we should think about how to add them to the component model since these are good for land-only hierarchical testing
  • we have produced the tiled versions of the soil color data and are populating the default data locations

@uturuncoglu
Copy link
Collaborator

@barlage Okay. As I know CCPP dropped the optional argument support but maybe my knowledge is old about it. Anyway, then we don't need tho provide those. Thanks for the information.

@HelinWei-NOAA
Copy link
Collaborator Author

@uturuncoglu I never ran component land RTs before. Where can I find some instructions? Thanks.

@HelinWei-NOAA As we discussed with @barlage, I have just started to work on component version of the NoahMP model to reflect these changes in https://github.com/NOAA-EMC/noahmp side. Did you tried to run component land RTs before? I am expecting that they will fail with this version of model. Anyway, once I have mods for the component model, I'll let you know.

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA I am workin on component model to bring these changes there. If you want to run it, you could use land component specific RTs such as control_p8_atmlnd_sbs. I am expecting this will fail without bringing these changes to the component model.

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA @barlage When I try to run the control_p8_atmlnd_sbs test, I am getting following error from rt.sh. Maybe the files are not staged to the right place. Any idea?

cp: cannot stat '/scratch2/NCEPDEV/stmp1/Sanath.Kumar/repo_viirs30s_bnuv2/sfc.C96/*color*': No such file or directory

@HelinWei-NOAA
Copy link
Collaborator Author

@uturuncoglu Are you running on hera? I can find those soil color data under /scratch2/NCEPDEV/stmp1/Sanath.Kumar/repo_viirs30s_bnuv2/sfc.C96/ on hera.

@HelinWei-NOAA @barlage When I try to run the control_p8_atmlnd_sbs test, I am getting following error from rt.sh. Maybe the files are not staged to the right place. Any idea?

cp: cannot stat '/scratch2/NCEPDEV/stmp1/Sanath.Kumar/repo_viirs30s_bnuv2/sfc.C96/*color*': No such file or directory

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA No. I could access only to Cheyenne and Orion but I prefer to have them on Cheyenne since this is main development environment for me. Could you copy those files to one of the platform? BTW, we are using those files also for GitHub action and they need to be available through cloud (S3 bucket). Otherwise, the GitHub action will fail.

@HelinWei-NOAA
Copy link
Collaborator Author

@uturuncoglu I don't have an account on Cheyenne. I have made a copy of the data on orion (/work/noaa/stmp/hwei/repo_viirs30s_bnuv2).

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA Thanks. I got them. Let me try to run the case with this data.

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA Okay. I could able to use the data. Probably, you might know but the directory is hard coded in the tests/fv3_conf/control_run.IN. BTW, do we need to update all the surface data to use new soil color data. I am asking because the component land model creates its mask by using orography file and as you can see from the attached plot from tile 1, I have missing values (-999 by default) around coastline. The component model also crashes after writing initial conditions and might be related with the inconsistency between oro data and new soil color. I am not seeing any issue in such as soil type data. Let me know what you think? BTW, I did not do anything else in terms of input files. Just point my directory in tests/fv3_conf/control_run.IN that has soil color data.

Screenshot 2023-06-13 at 2 30 14 PM

@barlage
Copy link
Collaborator

barlage commented Jun 13, 2023

@uturuncoglu I believe your tests probably use the fractional grid oro files so you have to use:

.../repo_viirs30s_bnuv2/sfc.C96.mx100/

@uturuncoglu
Copy link
Collaborator

uturuncoglu commented Jun 13, 2023

@barlage Okay. It is good to know. I could point the directory manually and test it but do we need to implement anything in tests/fv3_conf/control_run.IN. I think once this dataset staged into the common directory, the UFS Weather model will pick the right one. Right?

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA @barlage Okay. I fixed it. Here is the PR in component side,

NoahMP Component Model: NOAA-EMC/noahmp#7

@HelinWei-NOAA, you could list this PR in your top level UFS PR as dependency. I also have fork for UFS level that updates the NOAHMP-Interface since model files are renamed to use capital latter in their file extensions. This fork can be found in following link,

feature/hr2_land1 branch in https://github.com/uturuncoglu/ufs-weather-model.git

You could merge this branch with your top level UFS fork. Let me know if you need help.

@uturuncoglu
Copy link
Collaborator

uturuncoglu commented Jun 14, 2023

@HelinWei-NOAA I am also getting following errors under GNU (in our CI test defined on NoahMP repository),

/home/runner/work/noahmp/noahmp/app/noahmp/src/module_sf_noahmplsm.F90:1:0:

    1 | #define CCPP
      | 
Warning: "CCPP" redefined
<command-line>:2:0:

note: this is the location of the previous definition
/home/runner/work/noahmp/noahmp/app/noahmp/src/module_sf_noahmplsm.F90:11522:38:

11522 |         psimk=2.*alog(0.5*(1+x))+alog(0.5*(1+x*x))-2.*atan(x)+2.*atan1
      |                                      1
Error: In call to ‘alog’ at (1), type mismatch in argument ‘x’; pass ‘REAL(8)’ to ‘REAL(4)’
/home/runner/work/noahmp/noahmp/app/noahmp/src/module_sf_noahmplsm.F90:10772:48:

10772 |                       ust,vkc,1.0,iz0tlnd,0,0.0)
      |                                                1
Error: Type mismatch in argument ‘landsea’ at (1); passed REAL(4) to REAL(8)
/home/runner/work/noahmp/noahmp/app/noahmp/src/module_sf_noahmplsm.F90:10778:55:

10778 |                 call garratt_1992(zt,zq,znt,restar,1.0)
      |                                                       1
Error: Type mismatch in argument ‘landsea’ at (1); passed REAL(4) to REAL(8)
/home/runner/work/noahmp/noahmp/app/noahmp/src/module_sf_noahmplsm.F90:10787:45:

10787 |                          ust,vkc,1.0,0,0,0.0)
      |                                             1
Error: Type mismatch in argument ‘landsea’ at (1); passed REAL(4) to REAL(8)
/home/runner/work/noahmp/noahmp/app/noahmp/src/module_sf_noahmplsm.F90:4689:32:

 4689 |                        z0mo,z0h)
      |                                1
Error: Type mismatch in argument ‘vaie’ at (1); passed REAL(4) to REAL(8)
/home/runner/work/noahmp/noahmp/app/noahmp/src/module_sf_noahmplsm.F90:3605:51:

 3605 |      if ( abs (sigma) < 1.e-6 ) sigma = sign(1.e-6,sigma)
      |                                                   1
Error: ‘b’ argument of ‘sign’ intrinsic at (1) must be the same type and kind as ‘a’
make[2]: *** [CMakeFiles/noahmp.dir/build.make:283: CMakeFiles/noahmp.dir/src/module_sf_noahmplsm.F90.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/noahmp.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
Error: Process completed with exit code 2.

So, I think that there are some data type inconsistency in those lines that needs to be fixed in CCPP/Physics. Once it is fixed in there, I could update the NoahMP code repository too.

@HelinWei-NOAA
Copy link
Collaborator Author

@uturuncoglu Fixed those issues. Thanks.

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA Thanks. I think I need to update the model in the NOAA-EMC/noahmp PR. Did you push those changes to the repo?

@HelinWei-NOAA
Copy link
Collaborator Author

Yes.

@HelinWei-NOAA Thanks. I think I need to update the model in the NOAA-EMC/noahmp PR. Did you push those changes to the repo?

@uturuncoglu
Copy link
Collaborator

@HelinWei-NOAA I am not seeing any update in UFS Weather Model level in terms of updating CCPP/Physics and NOAHMP component. I think those needs to be also pushed to UFS level, otherwise the RTs will fail.

@FernandoAndrade-NOAA
Copy link
Collaborator

FernandoAndrade-NOAA commented Jul 14, 2023

Running with hera-rt since baseline was manually created for 07/13

@zach1221
Copy link
Collaborator

zach1221 commented Jul 14, 2023

Ok, I've re-created the baselines and hrrr_control_intel matching was successful this time. I'll go ahead and make the change to rt.conf in this PR.

@BrianCurtis-NOAA
Copy link
Collaborator

@FernandoAndrade-NOAA I know I completed WCOSS2 baselines/RT. Do I need to do those over again on Acorn/WCOSS2?

@jkbk2004
Copy link
Collaborator

hrrr_control_qr

@BrianCurtis-NOAA Regarding hrrr_control_qr issue, we don't need to redo the tests once logs are passed ok. @zach1221 pushed a fix on rt.conf to avoid corruption from duplicative baseline copy situation.

@FernandoAndrade-NOAA
Copy link
Collaborator

FernandoAndrade-NOAA commented Jul 17, 2023

@FernandoAndrade-NOAA I know I completed WCOSS2 baselines/RT. Do I need to do those over again on Acorn/WCOSS2?

@BrianCurtis-NOAA That would be up to you if you'd like to run for Acorn/WCOSS2 or skip it. After deciding that we'd also like to include #1838 as it's a single line change for specifying build directory. I believe @jkbk2004 mentioned earlier that it tested fine. We can also discuss in the meeting shortly

@BrianCurtis-NOAA
Copy link
Collaborator

Just to document (thankfully i had my Acorn window open from the weekend)

+ echo '059 hrrr_control_intel failed in check_result'
059 hrrr_control_intel failed in check_result
+ echo '059 hrrr_control_intel failed in check_result'
+ read -r failed_test_name
+ echo 'hrrr_control_intel 059 failed in run_test'
hrrr_control_intel 059 failed in run_test
+ echo 'hrrr_control_intel 059 failed in run_test'
+ read -r failed_test_name
+ echo '060 hrrr_control_qr_intel failed in check_result'
060 hrrr_control_qr_intel failed in check_result
+ echo '060 hrrr_control_qr_intel failed in check_result'
+ read -r failed_test_name
+ echo 'hrrr_control_qr_intel 060 failed in run_test'
hrrr_control_qr_intel 060 failed in run_test
+ echo 'hrrr_control_qr_intel 060 failed in run_test'
+ read -r failed_test_name
+ echo '061 hrrr_control_decomp_intel failed in check_result'
061 hrrr_control_decomp_intel failed in check_result
+ echo '061 hrrr_control_decomp_intel failed in check_result'
+ read -r failed_test_name
+ echo 'hrrr_control_decomp_intel 061 failed in run_test'
hrrr_control_decomp_intel 061 failed in run_test
+ echo 'hrrr_control_decomp_intel 061 failed in run_test'
+ read -r failed_test_name
+ echo '062 hrrr_control_2threads_intel failed in check_result'
062 hrrr_control_2threads_intel failed in check_result
+ echo '062 hrrr_control_2threads_intel failed in check_result'
+ read -r failed_test_name
+ echo 'hrrr_control_2threads_intel 062 failed in run_test'
hrrr_control_2threads_intel 062 failed in run_test
+ echo 'hrrr_control_2threads_intel 062 failed in run_test'
+ read -r failed_test_name

@BrianCurtis-NOAA
Copy link
Collaborator

Seems the baselines were corrupt on Acorn similar to seen previously. This was adjusted and RT's pass.

@jkbk2004
Copy link
Collaborator

Ok! we can start merging process as all tests are completed.

@grantfirl
Copy link
Collaborator

Ok! we can start merging process as all tests are completed.

@jkbk2004 Please see ufs-community/ccpp-physics#78 (comment). Let me know if you still want to start the merge process and I'll push the button.

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Jul 19, 2023

Noah-MP and FV3 PRs were merged. New hashes are NOAA-EMC/noahmp@569e354 and NOAA-EMC/fv3atm@e7dc085

@HelinWei-NOAA
Copy link
Collaborator Author

@jkbk2004 I have pointed to the right fv3 and noah-mp repo and revert .gitmodules

@jkbk2004
Copy link
Collaborator

@HelinWei-NOAA can you resolve the conversations as well?

@HelinWei-NOAA
Copy link
Collaborator Author

@jkbk2004 What is the conversation issue?

@jkbk2004
Copy link
Collaborator

@jkbk2004 What is the conversation issue?

@HelinWei-NOAA you can click "view" button and resolve old conversations.
1777

@jkbk2004 jkbk2004 self-requested a review July 19, 2023 16:37
@HelinWei-NOAA
Copy link
Collaborator Author

@jkbk2004 Yes. It was resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated. hera-RT Run Hera regression testing jenkins-ci Jenkins CI: ORT build/test on docker container New Input Data Req'd This PR requires new data to be sync across platforms Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adding the new soil color data and updating some of other LSC datasets